Follow-up: tackle 5 of 6 post-refactor TODOs (motor_poles, fold events, field_names, pub-cleanup, dead_code)#6
Merged
Conversation
…ld_names, dead_code audit #5 motor_poles: SessionMeta gets `motor_poles: Option<u32>`. BF build populates it from the `motor_poles` header; AP/PX4/MAVLink leave None. analysis::fft::analyze_vibration_unified reads `session.meta.motor_poles.unwrap_or(14)` instead of hard-coding 14, so real-world FFT eRPM→Hz conversion uses the right pole count for BF logs (HD camera builds with 7-pole motors, 6-pole quads, etc.). #3 fold session events: `analysis::analyze` now folds Session.events (format-native: ModeChange, Armed/Disarmed, LogMessage, Failsafe, GpsRescue, Crash, Custom) into FlightAnalysis.events as FirmwareMessage variants with synthetic levels. Severities map through cleanly; mode/armed/custom states get "info"; failsafe/crash get "critical"; gps rescue gets "warning". Frontend timeline now sees state changes alongside behavior-detected events without a shape change. Added a fold regression test. #1 field_names cleanup: was a stub listing only "time" + "gyro[*]" + extras keys. Now iterates every populated typed slot — gyro/accel/ setpoint/attitude per-axis, motors[i] / erpm[i] per channel, rc sticks + throttle, vbat / current / rssi, all GPS columns, plus extras. CLI `info` against btfl_002.bbl now lists 27 fields (was 14). Used by `propwash dump` field name resolution and the WASM raw-data tab. #6 dead_code audit on analysis/util.rs: stripped the stale TODO banner. Strategy enum and resample/resample_zoh shims kept as public API with allow(dead_code) on the subset not currently called by analysis (StepFill/Nearest variants, the TimeSeries- taking shims) so the public surface stays available for future callers without the lint flagging it. Documentation updated to explain why each is kept. Tests: 150 lib (+1 fold test) + 21 integration + 20 CLI + 22 web + 7 fft. All green; clippy clean; fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rnal
Replaces the public stringly-typed Session::field(SensorField) bridge
with field_by_name(&str), the recommended entry point for tools that
take user-supplied field names at runtime (CLI dump, WASM raw-data
tab, WASM spectrogram). field() stays around as crate-internal for
the BF parser tables that key on SensorField, but is no longer part
of the public API.
Surface changes:
propwash-core public API:
- Session::field_by_name(&str) -> Vec<f64> (new) — recommended.
- Session::field(&SensorField) (now pub(crate)) — internal only.
- analysis::fft::compute_spectrogram now takes &[(&str, &str)]
(axis label, canonical Session field name) instead of
&[(&str, &SensorField)]. Routes through field_by_name internally.
- types::SensorField, MotorIndex, RcChannel demoted to pub(crate).
- types::Unit deleted (was unused — only referenced by the deleted
SensorField::unit() method).
- types::RcChannel::index() deleted (unused).
Migrations:
propwash-cli/src/main.rs:
- info command: hardcoded ERpm/GyroUnfilt field() lookups replaced
with typed checks (motors.esc.is_some() and an extras key).
- dump command: drops the SensorField::parse + field(field) two-step
in favor of field_by_name(name) + filter empty results.
propwash-web/src/lib.rs:
- get_dump and get_filter_config: field_by_name.
- get_spectrogram: builds (label, canonical) string pairs for the
new compute_spectrogram signature.
propwash-core/tests/perf.rs + examples/bench.rs: switch to
field_by_name with canonical field names ("gyro[roll]" etc.).
Tests: 150 lib + 21 integration + 20 CLI + 22 web + 7 fft. All green;
clippy clean; fmt clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SensorField, MotorIndex, RcChannel are now public (with FromStr, Serialize, Deserialize, and feature-gated tsify-next derives). Session::field(&SensorField) is the sole lookup-by-handle method; field_by_name is gone. WASM bridge takes a typed SensorFields newtype that crosses the wasm-bindgen boundary as a TS discriminated union (SensorField[]), generated automatically by tsify-next. JS callsites construct typed field handles via a new parseSensorField helper that mirrors the Rust parser. CLI dump enumerates names via field_names(), prefix-filters, then parses each survivor into SensorField (extras keys / attitude fall back to Unknown, which yields an empty column). compute_spectrogram takes typed (label, &[f64]) axes; perf + bench tests measure the zero-cost cast_slice path with a 1ms ceiling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five of the six in-code TODOs left over from the typed-Session refactor (PR #5). The sixth (typed PID-term traces in Session) is left for a focused follow-up since it's a Session schema change that only BF can populate.
Commits
9c1662f): motor_poles into SessionMeta, fold Session.events into FlightAnalysis.events, expand field_names to all populated typed slots, dead_code audit on resampling helpers.e677609): SensorField/MotorIndex/RcChannel/Unit recede into crate-internal. NewSession::field_by_name(&str)is the public field-by-string entry point.field()and SensorField are no longer part of the public surface.What's externally visible
SessionMetagainsmotor_poles: Option<u32>(BF populates it from headers; AP/PX4/MAVLink leave None).session.meta.motor_poles.unwrap_or(14)instead of hard-coded 14.FlightAnalysis.eventsnow includes folded Session events (mode changes, armed/disarmed, firmware messages, failsafes) asFirmwareMessagevariants. Frontend timeline sees state changes alongside behavior-detected events.Session::field_by_name(&str) -> Vec<f64>— recommended public API for tools that take user-supplied field names.Session::field(&SensorField)— nowpub(crate). External callers migrate tofield_by_name.analysis::fft::compute_spectrogramtakes&[(&str, &str)](label, canonical name) instead of&[(&str, &SensorField)].Session::field_names()is no longer a stub — it iterates all populated typed slots (gyro/accel/setpoint/attitude per axis, motors, eRPM per channel, RC sticks/throttle, vbat, current, rssi, all GPS columns, plus extras).types::Unitdeleted (was unused).types::SensorField,MotorIndex,RcChanneldemoted topub(crate).Migrations carried out
infocommand's hardcodedfield()lookups replaced with typed checks;dumpusesfield_by_name.get_dump,get_filter_config,get_spectrogramusefield_by_name.field_by_name.What's still TODO
pid_p/pid_i/pid_das TriaxialSeries) need to be added to Session, and BF parser needs to populate them. AP/PX4/MAVLink don't expose these. Worth its own PR with a Session schema discussion.Test plan
infoagainst btfl_002.bbl shows 27 fields (was 14 stub-listed before).(&str, &str)interface.🤖 Generated with Claude Code